fix(resource_github_organization_custom_properties): support bool default values by secustor · Pull Request #3572 · integrations/terraform-provider-github

secustor · 2026-07-23T13:28:51Z

This is a minimal fix for users of default values and is intentionally tries not to reach the scope of #3234

Closes: #3580

Resolves infinite drift if when using resource_github_organization_custom_properties of value_type true_false and default_value.

Infinite terraform plan:

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # github_organization_custom_properties.stedisentry_enabled will be updated in-place
  ~ resource "github_organization_custom_properties" "foo" {
      + default_value      = "true"
        id                 = "foo"
        # (6 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Before the change?

customProperty.DefaultValueString() will return "", nill because of https://github.com/google/go-github/blob/a3951f514384b06cc428f394672020c14f7efd84/github/orgs_properties.go#L59

resource "github_organization_custom_properties" "foo" {
  property_name = "foo"
  value_type    = "true_false"
  required      = true

  // currently there is an infinite drift setting creating an addition on each plan 
  default_value = "true" // `true` has the same behavior 
}

This forces following workaround

resource "github_organization_custom_properties" "foo" {
  property_name = "foo"
  value_type    = "true_false"
  required      = true

  default_value = "true"

  lifecycle {
    ignore_changes = [default_value]
  }
}

After the change?

With this instead customProperty.DefaultValueBool() is called and with that the return value is true, nill which will then correctly set in the state

Pull request checklist

Schema migrations have been created if needed (example)
Tests for the changes have been added (for bug fixes / features)
Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

Yes
No

…ault values

github-actions · 2026-07-23T13:29:02Z

👋 Hi, and thank you for this contribution!

This repo is maintained by GitHub and community members on a best-effort basis. We'll get to this as soon as we can.

You can help us prioritize by joining the discussion on open issues and PRs, sharing details on the changes you need, and reviewing other contributions.

_{🤖 This is an automated message.}

deiga · 2026-07-24T08:01:24Z

Please mark clearly which issue this is going to resolve!

secustor · 2026-07-24T08:46:20Z

Resolves infinite drift if when using resource_github_organization_custom_properties of value_type true_false and default_value.

I have been under the impression that the explanation above is clear enough, now there is also an example of the Terraform plan which will be generated on each plan and is not reconciled.

deiga · 2026-07-25T19:43:24Z

@secustor I meant which issue number/URL.
If no issue exists yet, please create one :)

deiga

Thanks for the effort so far!

Copilot

Pull request overview

These provider review instructions are being used.

Fixes perpetual drift for boolean organization custom-property defaults by correctly reading and stringifying boolean API values.

Changes:

Handles true_false defaults during state refresh.
Adds regression coverage for empty follow-up plans.
Documents unsupported multi_select defaults.

Finding: The new test uses legacy assertion APIs instead of ConfigStateChecks.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
`github/resource_github_organization_custom_properties.go`	Reads boolean defaults correctly.
`github/resource_github_organization_custom_properties_test.go`	Adds regression coverage.
`templates/resources/organization_custom_properties.md.tmpl`	Documents default-value limitations.
`docs/resources/organization_custom_properties.md`	Updates generated documentation.

Use ConfigStateChecks/ExpectKnownValue for the true_false default_value assertion instead of the legacy TestCheckResourceAttr Check, and note the multi_select limitation in the default_value schema Description. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

deiga · 2026-08-02T14:03:55Z

 			"default_value": {
 				Type:        schema.TypeString,
-				Description: "The default value of the custom property",
+				Description: "The default value of the custom property. Not supported for multi_select properties.",


Would it make sense to add a validation to not use default with multi-select?

My understanding is this would be breaking the setup for existing users which use the ignore_changes workaround 🤔

I'm not sure that breaking the wrong way would be a problem. But if we want to be careful we should at least introduce a warning for this case

What kind of warning do you want me to add?

I see different patterns here in the repo.

Logging with log.Printf("[WARN] ...")

Using diagnostics, though that would need more changes in that PR

or simply in the docs

diagnostics would be the best, let's go with that 🙏

Drop the redundant default_value state check (Terraform core already enforces consistency with config) and fold the drift regression guard into a PostApplyPostRefresh plan check on the apply step.

…context-aware CRUD Switches the resource to the CreateContext/ReadContext/UpdateContext/ DeleteContext and StateContext signatures used by most resources in the provider, so the CRUD functions receive a context and can return diagnostics instead of a bare error. Update now delegates to Create, which already upserts via PUT and reads the property back, removing a redundant second read.

…elect default_value GitHub returns the default value of a multi_select property as a list of strings, which cannot be represented by the string default_value attribute. The value is therefore not stored in state and every plan shows a change for default_value. Emit a warning diagnostic on create and update instead of rejecting the combination, so existing configurations relying on the ignore_changes workaround keep working.

stevehipwell · 2026-08-10T11:12:05Z

I think #3234 should fix this, but if not this change would need to be made on top of the other PR.

secustor · 2026-08-10T11:29:40Z

I think #3234 should fix this, but if not this change would need to be made on top of the other PR.

Potentially, I have referred this PR in the PR body.
The intention is to deliver a minimal fix until #3234 lands.

stevehipwell · 2026-08-10T14:46:48Z

@secustor I don't think it's worth fixing this resource given that it's going to be replaced, I'd rather incentivise people to remove -> import sooner than later.

fix(resource_github_organization_custom_properties): support bool def…

a75de62

…ault values

github-actions Bot added the Type: Bug Something isn't working as documented label Jul 23, 2026

deiga added the r/organization_custom_properties label Jul 28, 2026

deiga requested changes Jul 28, 2026

View reviewed changes

Comment thread github/resource_github_organization_custom_properties_test.go Outdated

Comment thread templates/resources/organization_custom_properties.md.tmpl

deiga requested a review from Copilot July 28, 2026 18:14

deiga added the vNext label Jul 28, 2026

Copilot started reviewing on behalf of deiga July 28, 2026 18:15 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

Comment thread github/resource_github_organization_custom_properties_test.go Outdated

secustor requested a review from deiga July 29, 2026 13:37

deiga requested changes Aug 2, 2026

View reviewed changes

test: assert empty post-refresh plan instead of a second plan-only step

adea79e

Drop the redundant default_value state check (Terraform core already enforces consistency with config) and fold the drift regression guard into a PostApplyPostRefresh plan check on the apply step.

secustor requested a review from deiga August 5, 2026 13:43

secustor added 2 commits August 10, 2026 12:50

Uh oh!

Conversation

secustor commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before the change?

After the change?

Pull request checklist

Does this introduce a breaking change?

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

deiga commented Jul 24, 2026

Uh oh!

secustor commented Jul 24, 2026

Uh oh!

deiga commented Jul 25, 2026

Uh oh!

deiga left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

deiga Aug 2, 2026

Choose a reason for hiding this comment

Uh oh!

secustor Aug 2, 2026

Choose a reason for hiding this comment

Uh oh!

deiga Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

secustor Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deiga Aug 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

stevehipwell commented Aug 10, 2026

Uh oh!

secustor commented Aug 10, 2026

Uh oh!

stevehipwell commented Aug 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

secustor commented Jul 23, 2026 •

edited

Loading

secustor Aug 8, 2026 •

edited

Loading